-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix icon scrolling new #6479
base: develop
Are you sure you want to change the base?
Fix icon scrolling new #6479
Conversation
Thanks for adapting the code. To be honest, I'm not 100% happy about this one :/ The toolbar changes its color to grey, while it uses a light tinted shade everywhere else. Also, the system bar becomes colored and then stays that way, even when other screens don't need it to be colored. I played around with Maybe we can try yet another thing. We could extend the image beyond the notification bar, so that it becomes more immersive and fills the whole screen. The problem with this is that we need to manually change the color of the notification icons (I think), and we need to be careful about changing the color when the drawer is opened. That sounds like a nightmare from a code structure point of view. Hmm, I'm not really sure what to do. I guess we need to discuss this at our next community call. |
Hey @ByteHamster, are there any news regarding this? Has the community call already taken place? |
Hi @JonathanZopf, There was a community meeting but we didn't get to discuss it, sorry. The next one would be (exceptionally) on the third Saturday of the month: 17 June, 18:00 CEST. Would you be able to join? Might be nice if you'd be involved in the discussion about your PR yourself ;-) More info on our website: https://antennapod.org/events/community-meeting
I think that indeed it should use the same colour as everywhere else. Not sure what the colour spec is there.
Testing the debug build, it doesn't consistently do this. As in: in the beginning I didn't have this, at some point I did, and after force-closing the app and relaunching it again it's back to normal. When opening the side menu, the Status bar does indeed become grey on top of the white side menu, which is also not really expected.
That would be nice indeed. I don't know how it works code-wise, but wouldn't it be possible to let the background expand indeed, set the app top bar and back to 'normal' when it scrolls up? Another thing that bothers me with this screen is that when you scroll up, you don't see the podcast title at all any-more. It would be great, if (part of) the podcast title would appear in the top bar. E.g. like in GitHub: VID-20230603-WA0001.mp4 |
Hi, Unfortunately I am already occupied on Saturday evening and can therefore not attend this meeting. The current solution to this issue is a rather "hacky" one. Perhaps I need to further investigate a more radical solution to this problem. If you prefer a solution similar to the Github example (which does not look bad at all), we would probably need to completely redesign the layout of this screen. This would of course need further time. I will look into it when I have time. I will probably create a different pull request when I have a new solution. |
This pull request has been mentioned on AntennaPod Forum. There might be relevant details there: https://forum.antennapod.org/t/monthly-community-call/1869/42 |
@keunes , @ByteHamster I fixed two major issues of the last commits:
|
Generally is this commit more a temporary fix. We would actually need someone redesigning the whole layout of those two fragments. I've looked in to this but it's way too complex for me since we'd have to not only modify the .xmls but also adapt the Java code accordingly. But that would be for another day and would be much better than my current fix on the long run. |
Hi,
This PR is the successor of my old pull request #6424. I decided to create new PR since the new fix has barely anything in common with the old one.
As said previously, this should fix #6274. I completely reworked the ToolbarIconTintManager for this fix. It should now directly display the toolbar (with background instead of showing white icons above white text, fixing the fatal flaw in the last PR. Do you find it better, @keunes?